Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(linksystem): add reification to LinkSystem #158

Merged
merged 1 commit into from
Apr 2, 2021

Conversation

hannahhoward
Copy link
Collaborator

Goals

Proposal for how to integrate ADL creation into the LinkSystem loading process

Implementation

Add an optional reifier into the link system process to create ADLs. The reason to do this is to capture the link
system itself when reification happens, in case it needs to be put into the node, which is often not
accessible by the time you have a node.

Another alternative would be to make this specific to
selector traversal, similar to LinkNodePrototypeChooser

For Discussion

I actually left off Storage as a concern entirely. The simple version would be to just make a NodeSubstrater function with a shape like this:

type NodeSubstrater func(ipld.Node) (ipld.Node, error)

But in reality a Store operation on an ADL Node built with some kind of ADL builder (think HAMT) actually probably stores SEVERAL blocks. I'm not sure the absolute best way to do that. Technically, such a NodeBuilder could store its "sub blocks" on NodeBuilder.Build... but that also feels a bit odd since one does not expect that operation to store

Another option would be for NodeBuilder to assemble a bunch of nodes as needed in a temporary store, and then for the Store to move it to the actual blockstore? Hard to say.

add an optional reifier into the link system process. The reason to do this is to capture the link
system itself when reification happens, in case it needs to be put into the node, which is often not
accessible by the time you have a node. another alternative would be to make this specific to
selector traversal, similar to LinkNodePrototypeChooser
@willscott willscott requested a review from warpfork April 1, 2021 01:40
@willscott
Copy link
Member

leaving storage off entirely for this pass seems like a good delineation to draw in helping focus this conversation.

@warpfork
Copy link
Collaborator

warpfork commented Apr 1, 2021

Hm, this is really interesting!

The example of applying this with the rot13adl demo is a little boring, because it's just unconditionally calling reify on the thing -- but I think I see where you're going with this.

One could use the LinkContext to make choices about where to apply an ADL based on content (like the parent path, or etc), or just inspecting the data in the Node for some pattern.

The NodeReifier function then handles both "the signalling problem" from that information (e.g. decide if there's an ADL at all) and "the implementation reference problem" (the answer is "whatever's linked by this function").

Nice.

@warpfork
Copy link
Collaborator

warpfork commented Apr 1, 2021

One thought I'm kind of turning over in my head if this seems like something that belongs attached statefully to the LinkSystem, or if it's something that would be cleaner as another variation of Load function with this NodeReifier as an argument. But that's probably more of a stylistic question than a practical functional one.

@mvdan
Copy link
Contributor

mvdan commented Apr 1, 2021

One could use the LinkContext to make choices about where to apply an ADL based on content (like the parent path, or etc), or just inspecting the data in the Node for some pattern.

I'm struggling to imagine how I would ever use this feature. Is what you say here a good example? For instance, with go-ipld-adl-hamt, I think I'd just always want to use the ADL's nodes, so extra logic doesn't seem necessary.

@warpfork
Copy link
Collaborator

warpfork commented Apr 2, 2021

The idea is that you could use the NodeReifier to decide what ADLs to use and when, even when in the middle of a traversal. So, you could write a NodeReifier that says "okay, when I'm in a path that ends with 'foosys/bar/hamt', i'm going to try to load that as if it's a HAMT ADL". And then you'd be able to walks over whole trees with traversal.* and it would automatically load the ADL for you ever time it encounters that pattern, without disrupting the walk.

Copy link
Collaborator

@warpfork warpfork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I've slept on this and I still basically like it. The example isn't very exciting and that could use work, but that can be future work, especially if you're already eager to start using this in downstream stuff. Let's go for it.

@warpfork
Copy link
Collaborator

warpfork commented Apr 2, 2021

Did an additional informal poll on Slack about this and it got only 🎉 and 👍 emoji, so, let's roll.

@warpfork warpfork merged commit 7406578 into master Apr 2, 2021
@mvdan mvdan deleted the feat/add-reification-to-link-system branch August 13, 2021 13:38
@aschmahmann aschmahmann mentioned this pull request Aug 23, 2021
62 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants